-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fluent API for effects (monkeypatch as methods) #1105
Conversation
``` In [2]: from moviepy.editor import * In [3]: len(dir()) 47 # was 58 ```
what happened here? you closed this in favor of something else? |
Not intentional, sorry! |
Oh, it closed itself when I deleted the v2 branch. Apparently there's no way to change the base now (???). You might have to make a new one, sorry. |
ack, no problem. |
I think it's all sorted now, you can ignore this little fiasco :) |
@tburrows13 do you have an opinion about this? It's much humble that what I had in mind, that basically was do this patching via a metaclass to avoid the need to import the "patched classes" from a different place (i.e avoid However I guess it's cleaner enough for now, and much more pythonic than the execs and The unique downside (very minor IMHO) is that fx namespaces changed if for some reason you import effects directly from the packages. In master if you import something from the In [1]: from moviepy.video.fx import mirror_x
In [2]: mirror_x?
Type: module
String form: <module 'moviepy.video.fx.mirror_x' from '/home/tin/lab/moviepy/moviepy/video/fx/mirror_x.py'>
File: ~/lab/moviepy/moviepy/video/fx/mirror_x.py
Docstring: <no docstring> and here you get the actual fx function for the same import In [3]: from moviepy.video.fx import mirror_x
In [4]: mirror_x?
Signature: mirror_x(clip, apply_to='mask')
Docstring: flips the clip horizontally (and its mask too, by default)
File: ~/lab/moviepy/moviepy/video/fx/mirror_x.py
Type: function However, this way still works in the new implementation In [1]: from moviepy.video.fx.mirror_x import mirror_x
In [2]: mirror_x?
Signature: mirror_x(clip, apply_to='mask')
Docstring: flips the clip horizontally (and its mask too, by default)
File: ~/lab/moviepy/moviepy/video/fx/mirror_x.py
Type: function so, we are mostly ok. |
This is definitely better than what we have currently, and better than #1104. Thanks for detailing how this change affects importing from the I wouldn't be opposed to getting rid of Perhaps that last paragraph should be a different issue/PR, and that last sentence would be quite a bit more of a drastic change and might require reworking the hierarchy of moviepy if everyone is accessing the classes from their proper locations. Furthermore, I still don't know if doing things in Your thoughts would be appreciated :) |
Closes #591 |
Ok, here's my current thinking. Move most of the standard imports and the fx 'trickery' into the main We can also look at including extra imports into either |
This PR is a (little bit) cleaner implementation to patch "functions effects" as methods. Supersedes #1104
Basically, avoid to use
exec
and deprecatesfx.all
modules, importing every function in each package's init.In addition, it defines
__all__
to reduce the namespace pollution when importfrom moviepy.editor import *